Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a signal handler for SIGINT when building a detector #1300

Closed
wants to merge 4 commits into from
Closed

Add a signal handler for SIGINT when building a detector #1300

wants to merge 4 commits into from

Conversation

jmcarcell
Copy link
Member

To cancel while building a geometry it seems I have to kill the process or the terminal where it's running. With this change it's possible to cancel by doing a Ctrl-C, and it seems to be working both in Python and C++:

import dd4hep
det = dd4hep.Detector.getInstance()
det.fromCompact(f'{os.environ["k4geo"]}/FCCee/CLD/compact/CLD_o2_v05/CLD_o2_v05.xml')

And then press Ctrl-C:

Compact          INFO  ++ Converted subdetector:LumiCalBackShield of type LumiCal_o1_v01 [calorimeter]
Calling Converter<Compact>::operator() with element: lccdd
Utilities        INFO  +++ setDetectorTypeFlags for detector: Vertex not set.
Compact          INFO  ++ Converted subdetector:Vertex of type DD4hep_SubdetectorAssembly
Compact          INFO  ++ Converted subdetector:VertexBarrel of type ZPlanarTracker [tracker]
XXX Vertex endcap layers: 6
Compact          INFO  ++ Converted subdetector:VertexEndcap of type VertexEndcap_o1_v06 [tracker]
Compact          INFO  ++ Converted subdetector:VertexVerticalCable of type TrackerEndcapSupport_o1_v02
VertexEndcap     INFO  +++ Processing SurfaceInstaller for subdetector: 'VertexEndcap'
DD4hep_GenericSurfaceInstallerPlugin: argument[1] = dimension = 2
DD4hep_GenericSurfaceInstallerPlugin: argument[2] = u_x = -1
DD4hep_GenericSurfaceInstallerPlugin: argument[3] = v_z = 1
DD4hep_GenericSurfaceInstallerPlugin: argument[4] = n_y = 1
DD4hep_GenericSurfaceInstallerPlugin: vectors: u( -1 , 0 , 0) v( 0 , 0 , 1) n( 0 , 1 , 0) o( 0 , 0 , 0)
Calling Converter<Compact>::operator() with element: lccdd
Utilities        INFO  +++ setDetectorTypeFlags for detector: InnerTrackers not set.
Compact          INFO  ++ Converted subdetector:InnerTrackers of type DD4hep_SubdetectorAssembly
^CCaught signal SIGINT, exiting...

BEGINRELEASENOTES

  • Add a signal handler for SIGINT when building a detector, allowing to cancel with Ctrl-C

ENDRELEASENOTES

@MarkusFrankATcernch
Copy link
Contributor

MarkusFrankATcernch commented Jul 22, 2024 •
edited
Loading

I am not a big friend of this PR. In general the rule is always:

  • Generic signals (such as SIGINT, SIGTERM, etc) are handled by applications, not by libraries.
  • Only specific signals are handled by libraries e.g. SIGIO in an I/O library.
  • libraries should generally avoid to call ::exit(). A library should only throw an exception or return a failure status.

In the event this behaviour is not wanted, users have to scan source code to find all signals handled internally.
This is not very convenient.

  • If really needed, one could have this behaviour in dd4hep.py.....
  • We could also have a plugin implementing the behaviour without the need to enforce the handler on everybody.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 22, 2024 •
edited
Loading

Test Results

   14 files     14 suites   5h 36m 36s ⏱️
  365 tests   365 ✅ 0 💤 0 ❌
2 510 runs  2 510 ✅ 0 💤 0 ❌

Results for commit d99f75b.

♻️ This comment has been updated with latest results.

@BrieucF BrieucF mentioned this pull request Oct 15, 2024
Closed
@MarkusFrankATcernch MarkusFrankATcernch mentioned this pull request Oct 17, 2024
Merged
@andresailer
Copy link
Member

Closing this PR in view of the changes from #1336. But we need to enable the signal handler in ddsim now, I think.

@MarkusFrankATcernch
Copy link
Contributor

@andresailer Yes. The handler would have to be enabled in ddsim. The test show which line to call:

geant4 = DDG4.Geant4(kernel, tracker='Geant4TrackerCombineAction')
geant4.registerInterruptHandler()

@MarkusFrankATcernch MarkusFrankATcernch mentioned this pull request Oct 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andresailer andresailer andresailer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmcarcell @MarkusFrankATcernch @andresailer